I'm not sold on this change yet... I agree we shouldn't assume anything thrown/caught is an Error object and the unknown type is best for this purpose because it forces us to do proper validation in the catch clause. But this change barely gets us halfway there: it checks that the property exists but it doesn't help with type-safety.
I don't think a generic lib file can solve this problem on its own. I think this sort of validation should be limited to its respective catch clause. For example here this could look like:

import { HttpErrorResponse } from '@angular/common/http';

try {
	// ...
} catch {
	if (error instanceof HttpErrorResponse) {
		const status = error.status;
//			^? number
	}
}

🙌 This is just the kind of push back I was hoping for! I am very interested in digging in deeper here to find the best solution.

The challenge I see with doing something like error instanceof HttpErrorResponse in existing code is that we would have to be absolutely certain of what is in scope for the try-block and what kind of errors might be thrown. Taking this code as a good example, there is quite a bit of logic wrapped in the try-block and I am not confident that anything that was being thrown with a status property, is definitely going to be instanceof HttpErrorResponse. For example, even with a local Pouch db, if you call get with a docId that does not exist, it will throw an error with status: 404, but I do not think it will be a HttpErrorResponse...

This was the main reason I settled on this half-baked pattern matching approach. 😬

his change barely gets us halfway there: it checks that the property exists but it doesn't help with type-safety.

100% agree. But, I really think that if we are going to do nested type-checking, we should revisit the idea of adding a schema validation library.... 😬 I have found effect/schema to be super powerful in chtoolbox and I think we could reduce a lot of manual logic that we have by pulling in something like that. (FTR, not saying we go with effect/schema here in cht-core.... 😅 Zod, seems like the default option, but I find https://github.com/fabian-hiller/valibot to be very interesting as well, particularly with its claims of superior tree-shaking....)

Did a simple side-by-side comparison of zod vs valibot for evaluating the schema of this error. The code is nearly identical:

zod:

      const MyErrorSchema = z.object({
        status: z.number().optional(),
      });
      const myError = MyErrorSchema.parse(error);

valibot:

      const MyErrorSchema = v.object({
        status: v.optional(v.number()),
      });
      const myError = v.parse(MyErrorSchema, error);

Then, I used npm run build to build the prod assets (with tree-shaking) and compared the sizes of api/build/static/webapp:

• Current: 8384K
• Zod: 8444K - +60K
• Valibot: 8388K - +4K

This is inline with valibot's claims to be significantly more efficient than Zod because the structure of the project is optimized for tree shaking.

IMHO +4K is worth it for sane schema validation. In reality, we should be able to replace a lot of janky manual code checks with valibot.

I think Mokhtar does have a point in finding the object type of err. It is the general pattern in other programming languages as well. For eg:

1. Java

    try {  
        // Code that may throw an exception  
    } catch (IOException e) {  
        // Handle IOException  
    } catch (NumberFormatException e) {  
        // Handle NumberFormatException  
    } catch (Exception e) {  
        // Handle any other exceptions  
    }  

2. python

try:
    n = 0
    res = 100 / n
    
except ZeroDivisionError:
    print("You can't divide by zero!")
    
except ValueError:
    print("Enter a valid number!")
    
else:
    print("Result is", res)
    
finally:
    print("Execution complete.")

I think we should apply these here as well and I agree that we probably won't know what kind of error the error object is. We would check for the most likely errors in the conditionals. If we are not sure what the error object would be a type of then, the getProperty function could be used in the default clause. For eg: when making a network call to an endpoint it's most likely not gonna throw a FileNotFound kind of error.

I'm not sure if zod and valibot are required here.